Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Making structure pattern more flexible to customization options #618

Merged
merged 8 commits into from
Mar 16, 2016

Conversation

metatoaster
Copy link
Member

This involves changes so that the actions don't make assumptions on the availability on certain key/value/attributes that are quite specific to folder_contents, grant full control over what columns are available and ensure that the settings can be made specific to one view only and not clobber others.

Done in response to issue #616.

}
if (self.options.urlStructure) {
var url = self.options.urlStructure.base + path + self.options.urlStructure.appended;
window.history.pushState(null, null, url);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're not checking for availability of window.history.pushState here, like it was done before. Therefore loosing compatibility with IE < 10. (IIRC, we wanted to support IE>=8)
no sorry, didn't see the line above.

metatoaster and others added 8 commits March 16, 2016 15:51
- New base test description for structure
- This one will contain customization and have no data (to verify some
  basic implementation details).
- Changed setup for first test to be JSON format for better syntax
  visualization.
- Empty body.html added to afterEach for cleanup (test teardown).

Paste button no longer required to exist.

- Ensure that a missing paste button will prevent the table from being
  rendered.

Ability to customize the cookie for columns

- Define a new key "activeColumnsCookie" for structure.
- This enables view specific column settings (i.e. a view on a list of
  users may have sufficiently different columns such that the settings
  for folder_contents shouldn't apply or interfere with at all).
- Needed adjustments in the view and template for tablerow to prevent
  rendering from being aborted to missing data.
- Still need a bare minimum set of attributes being available (i.e.
  Title and getURL).

Formalize definition of nested params in structure

- Provide default parameters with the _default_ prefix.
- Replace the demoButton with _default_buttons to be consistent.
- Provide a generalized helper function for the replacement of the
  object/array type options for the structure pattern.
- First usage on this is to write more tests, in this case document that
  the buttons can be completely replaced, or even completely removed.
Test for per item dropdown action menu click

- Ensure that at least one of them works (move-top).
- See that the defaults will continue to work when changes to migrate
  generation of the items and bindings are moved to a more configurable
  definition.
- Make it actually do the full round trip including the existing button.

Initial split of Actions from ActionMenu

- Per-item action methods should not be defined directly within the view
  class; view class should only deal with rendering/registration of the
  events, and those events should be registered within a seaparate model
  which this view class should make use of
- Baby steps: want to make it possible to hook in events that could be
  defined by other libraries.

Hooking in the event with requirejs

- Provide a way to specify dynamically where the modules are for a given
  event along with the label.

Configurable menu options.

- Move option visibility logic out of template
- Instead, construct the table for this shared between the event method
  and the template.
- Enable the user-specifiable option for the template.
- Include a test for the customization of action menu.
- Pass down menu option into current folder actions

Have action menu tests define/use custom modules

- Show that the internal procedure of requiring/using modules loaded via
  requirejs functions as expected.

Hook moveUrl to the availability of rearrangement

- Don't apply the rearrange class to the parent of the table if that was
  not provided in the constructor.
- Also don't provide the move-top and move-bottom actions for that case.

Ability to specify action at the table level

- Add in tests to ensure correctness of existing work.
- Still need to figure out how to do this at the per item level.

Various per item actionmenu tests

- Fixed the missing entries when used (have to clone that object because
  javascript).
- Actually add in the tests for the functionalities for those actionmenu
  items to ensure that they are triggered to ensure they stay correct.
- Verify certain xhr requests.

Split up the navigation stuff

- getWindow now a utility function - make it stubbable because assigning
  window.location will result in test framework triggering a disconnect.
- Have the new module that only deal with general navigation such as
  opening a link.

User provided link click handlers

- The actions for handling folder and standard item clicks can be
  defined similar to how actionmenu was done.
- Removed all the extraneous methods that were duplicated for this in
  the previous commit(s).

Do a proper integration test for actionmenu.

- Show that it works from within a table also.

Split up ActionMenu into view and menu component

- Renamed existing ActionMenu to ActionMenuView (maintaining consistency
  with other js files).
- Split out the default ActionMenu generation functionality into its own
  module.

Assign menuOptions iff menuGenerator return object

- Prevent some basic configuration failure cases - at least show data
  even if action menu might be missing.

Finalize menuGenerator integration

- This allow the generator to be specified at the app level
- Test demonstrates how menu can look into each model for given row to
  generate the appropriate actions.

Check that unselect actually unselect stuff

- Find out that if the raw data is missing the id attribute for a given
  item, the de-selection will not be able to unselect the checkbox.

Testing selectAll within a folder in table

- Show that this functionality also work, but the intial test data had
  to be manipulated
- Also had to reset the queryHelper, because the currentPath is not
  properly cleansed between tests.
- Tests that relied on the above had to be adjusted to go back down to
  the expected currentPath.
- This was done to clarify the scope and hierarchy of the affected
  objects.

- The AppView instance is where the path setting should be done, even
  though it ultimately pokes into the methods by the same name within
  ResultCollection.

  - Code that previously used queryHelper.currentPath now delegate this
    through the methods provided by AppView.

- Formalizing what needs to be implemented for custom ResultCollection
  models - it needs to provide getCurrentPath/setCurrentPath along with
  the relevant Backbone.Paginator.requestPager methods.  This should be
  documented.

  - The instance of AppView.collection now keeps track of the path that
    the AppView should be on - no longer requiring fetching this value
    from queryHelper

- A completely custom ResultCollection can be now provided.  It does not
  need to follow the existing usage of the QueryHelper/Parser class as
  custom implementations don't need to deal with the Plone Catalog.

- Default ResultCollection implementation now constructs queryParser and
  queryHelper inside itself to keep as much of the Plone catalog and
  content type specific information out of the pattern itself to aid in
  allowing reuse without the unwanted Plone bits (specifically with the
  queryHelper stuff, it really had no place on the top level option
  object).

- Default implementation of the queryParser and construction of
  queryHelper should remain unchanged except for its references to
  certain attributes that exist on the parent app objects (such as the
  AppView.toolbar).

Formalize the previous changes with a final test.

- Created a test that cover the case where the QueryHelper (and
  collection.queryParser) is triggered, such that they together should
  generate the correct query parameters for consumption by the
  vocabulary endpoint within a Plone instance.

- The test also serves to finalize the clarified version of the object
  relationships between the structure pattern, AppView, ResultCollection
  and QueryHelper.  The previously tangled web of relationships
  (specifically the setting/access of the current path to be
  accessed/represented by the structure view) is more controlled.

- To really hammer in this point, AppView has an instance of
  ResultCollection, and it gained complete ownership of QueryHelper as
  it utlimately controls what gets populated inside the table, despite
  how QueryHelper needs to have access to the certain attributes of the
  pattern itself.

Ensure that the selectAll option work still

- Moving the queryHelper into the collection broke that, turns out this
  wasn't tested before.
- For the link that opens a new window, there will be cases where the
  view suffix is not desirable.  This allows the data source to provide
  an explicit one.  Fallback behavior is what it was before.
- This change ensures that it is possible to trigger the events off the
  correct base url as the views will not have the actual real views that
  would have been resolved by the event.
- Ensure that the push/popstates are properly handled/triggered.

Also more tests for window pushState

- Added the configuration needed to trigger that.
- Added the required window mocks to allow the pushState to be tested.
- Ensure that the URL argument passed is as expected.
- Provide an alternative way to set the pushState url, using a format
  string that make use of the {path} token for substitution as an
  alternative to the somewhat more opaque urlStructure parameter.
@thet thet force-pushed the metatoaster-structure-master branch from 69712c9 to 4fa0384 Compare March 16, 2016 14:53
thet added a commit that referenced this pull request Mar 16, 2016
WIP: Making structure pattern more flexible to customization options
@thet thet merged commit ba76b86 into master Mar 16, 2016
@thet thet deleted the metatoaster-structure-master branch March 16, 2016 14:53
@thet
Copy link
Member

thet commented Mar 16, 2016

Tnx for the work on structure pattern!

@metatoaster
Copy link
Member Author

Thanks for doing the review and the merge.

If the pushStateUrl way of formatting the pushState are going to be supported (in 43b236d), the previous way with the urlStructure could probably be deprecated in the future.

karalics pushed a commit to karalics/mockup that referenced this pull request Apr 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants